Repository navigation
Conversation
|
Auto-sync is disabled for draft pull requests in this repository. Workflows must be run manually. Contributors can view more details about this message here. |
📝 SummarySummary by CodeRabbit
WalkthroughAdds two manually triggered workflows. One validates a pull request before dispatching a private smoke workflow. The other validates a public CI run and its Windows Arm64 artifacts before dispatching private validation. Both verify the returned private workflow run. ChangesCross-repository PR smoke
Private WoA validation
Priority: ⬇️ Low Change: Feature Merge Risk: 🟡 Moderate · up to Protect the private-dispatch credential and fix both run checks before merging; either manual broker can report failure after starting its private run.
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/woa-private-dispatch.yml (1)
67-83: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winsuggestion: Derive the CUDA build version from
ci/versions.ymlatpublic_sha. Do not hard-code13.4.2.Two values are fixed to
13.4.2:
- the producer job name
Build win-arm64, CUDA 13.4.2 / py3.13;- the artifact name
cuda-bindings-python313-cuda13.4.2-win-arm64-<sha>.In
.github/workflows/ci.yml, both values come from.cuda.build.version(Lines 224-242 and 444-467). After the next CUDA version bump, this broker will reject every valid public run until someone edits this file. The failure is safe, but the broker becomes unusable.Fix: read
ci/versions.ymlatpublic_shawithgh api repos/NVIDIA/cuda-python/contents/ci/versions.yml?ref=$public_sha. Then build the job name and the artifact names from.cuda.build.version.
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
3bdf7831-9285-4563-bf65-3d0b14907b12
📒 Files selected for processing (2)
.github/workflows/woa-crossrepo-smoke.yml.github/workflows/woa-private-dispatch.yml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
|
|
The PR needs manual trigger to run smoke test and dispatch-validate test, if I understand correctly. Are we able to test the manual trigger when the PR is open, or we need to merge the PR to allow manual trigger? |
|
The PR introduces a two-phase design: a lightweight smoke check between the public and private repos to cheaply validate cross-repo connectivity/config, followed by a dispatch-validate step that verifies a merged main build and triggers real GB10 run. This lets connectivity/config issues surface cheaply in the first phase, before the costlier validation phase. LGTM. I just have a few questions. |
There's a way, as pointed out here: https://github.com/NVIDIA-dev/cuda-python-private/pull/615#issuecomment-5797975839 But there's still a gap: |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
- Configuration used: Repository: NVIDIA/cuda-python/.coderabbit.yaml
- Review profile: CHILL
- Plan: Enterprise
- Run ID:
f257626e-4b8d-488c-bad5-26dc8c1e75d6
📒 Files selected for processing (2)
.github/workflows/woa-transport-check.yml.github/workflows/woa-validation-dispatch.yml
Included review availability: This review used your included allowance. Your plan provides up to 12 included reviews per hour; 11 remain after this review.
lijinf2
left a comment
lijinf2
left a comment
There was a problem hiding this comment.
LGTM.
.path mismatched issue raised by coderabbit seems not blocking merge. We can confirm whether that is an actual issue after merge.
So we will have two new rows under Github Action. One is "woa-transport-check", and the other is "woa-validation-dispath". Does "woa-validation-dispath" changes status (i.e. green or red) before the private run finished or after?
The check run will get updated depend on the stage of the private run. For example, after the dispatch is admitted, the private run will create a POST to update the check with "in progress" status. And once test finished, the report job will update the check with |
| (.head.sha | test("^[0-9a-f]{40}$")) | ||
| ' <<< "$pr" >/dev/null | ||
| public_sha=$(jq -r .head.sha <<< "$pr") | ||
| correlation_id="smoke:381173759:$PR_NUMBER:$public_sha:${GITHUB_RUN_ID}:${GITHUB_RUN_ATTEMPT}" |
There was a problem hiding this comment.
Q: I assume the magic number here is the repo unique ID? In any case, why do we need that?
There was a problem hiding this comment.
Yeah, fwiw I think agent is actioning in panic: repo name can change, but unique-id doesn't. But I agree using magic number here is over-engineering.
| inputs: { | ||
| schema_version: "1", | ||
| public_repository: "NVIDIA/cuda-python", | ||
| public_repository_id: "381173759", |
There was a problem hiding this comment.
ditto, can we avoid hard-coding the magic number (given that the above line is also already hard-coding the repo name -- which can be also relaxed or replaced by e.g. ${{ github.event.repository.name }})?
There was a problem hiding this comment.
FWIW, the workflow is already hardened by if: ${{ github.repository == 'NVIDIA/cuda-python' }}.
| workflow_dispatch: | ||
| inputs: | ||
| pr_number: | ||
| description: Open same-repository PR to receive the smoke Check Run | ||
| required: true | ||
| type: number |
There was a problem hiding this comment.
Q: I thought the security requirement is that we should not dispatch any un-merged PRs? How about taking no inputs and always dispatching from main for now?
There was a problem hiding this comment.
This woa-transport-check is a smoke run, on the private side it doesn't trigger gb10 runners. It's only used to validate the github app works. The reported result will be posted on the PR's check run:
https://github.com/NVIDIA/cuda-python/pull/3025/checks
I think it's chosen this way to avoid polluting any existing commits that's already merged to main.
There was a problem hiding this comment.
In the future PR this workflow is planned to be removed.
| -H 'X-GitHub-Api-Version: 2026-03-10' \ | ||
| "repos/NVIDIA-dev/cuda-python-private/actions/runs/$private_run_id") | ||
| jq -e ' | ||
| .repository.id == 809898190 and |
There was a problem hiding this comment.
ditto, maybe if we can just use the repo name instead of unique ID? (The name is already used above for building response anyway.)
| on: | ||
| workflow_dispatch: | ||
| inputs: | ||
| public_run_id: | ||
| description: Successful public main CI run containing the exact WoA artifacts | ||
| required: true | ||
| type: string |
There was a problem hiding this comment.
nit: For convenience we can make this optional. We have a lookup-run-id script that can look up the latest run ID, see ci-nightly.yml:
cuda-python/.github/workflows/ci-nightly.yml
Lines 25 to 32 in 5fe0749
| jq -e ' | ||
| .repository.id == 381173759 and | ||
| .workflow_id == 155304118 and | ||
| .path == ".github/workflows/ci.yml" and | ||
| .event == "push" and | ||
| .head_branch == "main" and | ||
| .status == "completed" and | ||
| .conclusion == "success" and | ||
| (.head_sha | test("^[0-9a-f]{40}$")) | ||
| ' <<< "$run" >/dev/null | ||
| public_sha=$(jq -r .head_sha <<< "$run") |
There was a problem hiding this comment.
ditto, this can be retrieved by passing --head-sha to lookup-run-id
| artifacts-json: ${{ steps.resolve.outputs.artifacts_json }} | ||
| correlation-id: ${{ steps.resolve.outputs.correlation_id }} | ||
| public-producer-job-id: ${{ steps.resolve.outputs.public_producer_job_id }} | ||
| public-run-attempt: ${{ steps.resolve.outputs.public_run_attempt }} |
There was a problem hiding this comment.
We have enough sophisticated artifact passing across pipelines, but so far I don't see any need for run_attempt (because multiple attempts share the same run ID). Is it really needed?
There was a problem hiding this comment.
(ditto for public_producer_job_id)
| expected_names=$(jq -n \ | ||
| --arg sha "$public_sha" '[ | ||
| "cuda-pathfinder-wheel", | ||
| ("cuda-bindings-python313-cuda13.4.2-win-arm64-" + $sha), | ||
| ("cuda-core-python313-win-arm64-" + $sha) | ||
| ]') | ||
| artifacts_json='[]' | ||
| while IFS= read -r artifact_name; do | ||
| response=$(gh api --method GET \ | ||
| -H 'X-GitHub-Api-Version: 2026-03-10' \ | ||
| "repos/NVIDIA/cuda-python/actions/runs/$PUBLIC_RUN_ID/artifacts" \ | ||
| -f name="$artifact_name" \ | ||
| -F per_page=100) | ||
| artifact=$(jq -c -e \ | ||
| --arg name "$artifact_name" ' | ||
| if .total_count == 1 and | ||
| (.artifacts | length) == 1 and | ||
| .artifacts[0].name == $name and | ||
| .artifacts[0].expired == false | ||
| then .artifacts[0] | {id, name, digest} | ||
| else error("expected exactly one unexpired artifact named " + $name) | ||
| end | ||
| ' <<< "$response") | ||
| artifacts_json=$(jq -c \ | ||
| --argjson artifact "$artifact" \ | ||
| '. + [$artifact] | sort_by(.name)' <<< "$artifacts_json") | ||
| done < <(jq -r '.[]' <<< "$expected_names") | ||
| jq -e ' | ||
| length == 3 and | ||
| all(.[ ]; | ||
| (.id | type == "number") and | ||
| (.digest | test("^sha256:[0-9a-f]{64}$"))) | ||
| ' <<< "$artifacts_json" >/dev/null |
There was a problem hiding this comment.
Q: I am a bit confused by this code. Is artifacts_json a hard requirement? I think providing just the repo name and the run ID is enough to retrieve artifacts, akin to gh run download ...? All we need on the private repo side is a GH token that has actions:read.
This branch has not been deployed
3 participants
Description
Tracks https://github.com/NVIDIA-dev/cuda-python-private/issues/584.
This is public PR A of the four-PR WoA cross-repository CI rollout. It adds two manually dispatched brokers and enables no automatic
maintrigger:mainCI run, its producer jobs, SHA ancestry, artifact names, IDs, and server digests before dispatching private validation.The private-dispatch App credential is referenced only by isolated jobs with
permissions: {}. Those jobs do not check out source or download artifacts.Depends on private foundation PR https://github.com/NVIDIA-dev/cuda-python-private/pull/650.
Rollout
After both foundation PRs merge:
WoA cross-repository transport smokeagainst a public draft PR and verifycuda-python WoA integration / transport smokecompletes neutral without a GB10 job.WoA exact public-main dispatchwith known eligible run37333876562and verify the canonical check plus the minimal GB10 import test.Validation
actionlintpasses for both added workflows.git diff --checkpasses.37333876562.Checklist